Reviewable status: 0 of 7 files reviewed, 2 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel)

a discussion (no related file):
This PR is based on top of #1973, thus requires that one to be merged first.

libos/src/libos_init.c line 303 at r1 (raw file):

        return 0;

    stack_size = ALLOC_ALIGN_UP(stack_size);

Note that stack size doesn't need to be aligned when setting the resource limit RLIMIT_STACK. That's what Linux does -- it doesn't silently adjust the stack size, instead Linux just writes whatever stack size the user supplies to it.

Thus I moved this alignment of the stack further down in this function.

Reviewed and looks good to me.

Reviewed 7 of 7 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: Intel)

Thanks @efu39 !

Reviewable status: 2 of 7 files reviewed, 1 unresolved discussion, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @efu39)

a discussion (no related file):

Done, rebased

Reviewed 2 of 7 files at r1, 5 of 5 files at r2, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @dimakuv)

libos/test/regression/rlimit_stack.c line 5 at r2 (raw file):

#include <errno.h>
#include <fcntl.h>

Do we need this?

Code quote:

#include <fcntl.h>

libos/test/regression/rlimit_stack.manifest.template line 16 at r2 (raw file):

sgx.debug = true
sgx.edmm_enable = {{ 'true' if env.get('EDMM', '0') == '1' else 'false' }}
sgx.use_exinfo = {{ 'true' if env.get('EDMM', '0') == '1' else 'false' }}

Do we need this?

Code quote:

sgx.use_exinfo = {{ 'true' if env.get('EDMM', '0') == '1' else 'false' }}

Reviewable status: 5 of 7 files reviewed, 3 unresolved discussions, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: Intel), "fixup! " found in commit messages' one-liners (waiting on @efu39 and @kailun-qin)

libos/test/regression/rlimit_stack.c line 5 at r2 (raw file):

Done.

libos/test/regression/rlimit_stack.manifest.template line 16 at r2 (raw file):

Done. You're right, not required.

Reviewed 2 of 2 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion, not enough approvals from different teams (1 more required, approved so far: Intel), "fixup! " found in commit messages' one-liners

Reviewed 5 of 5 files at r2, 2 of 2 files at r3, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions, not enough approvals from different teams (1 more required, approved so far: Intel), "fixup! " found in commit messages' one-liners (waiting on @dimakuv)

libos/test/regression/rlimit_stack.c line 18 at r3 (raw file):

    CHECK(getrlimit(RLIMIT_STACK, &rlim));
    printf("old RLIMIT_STACK soft limit: %lu\n", (uint64_t)rlim.rlim_cur);
    uint64_t old_lim = (uint64_t)rlim.rlim_cur;

Just delete this and use rlim.rlim_cur directly in the IF?

libos/test/regression/rlimit_stack.c line 21 at r3 (raw file):

    /* make sure we can increase the current soft limit */
    if (old_lim >= (uint64_t)rlim.rlim_max)

Why the cast here?

Code quote:

(uint64_t)

libos/test/regression/rlimit_stack.c line 39 at r3 (raw file):

        /* NOTE: we currently don't test that the stack limit is indeed enforced */
        exit(0);
    } else {

Why is some part of the code inside else block and some outside? Both always execute together.

libos/test/regression/test_libos.py line 1103 at r3 (raw file):

        self.assertIn("TEST OK", stdout)

    def test_165_rlimit_stack(self):

Or maybe I'm missing something?

Suggestion:

def test_162_rlimit_stack(self):

Reviewable status: 5 of 7 files reviewed, 5 unresolved discussions, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: Intel), "fixup! " found in commit messages' one-liners (waiting on @kailun-qin and @mkow)

libos/test/regression/rlimit_stack.c line 18 at r3 (raw file):

Done.

libos/test/regression/rlimit_stack.c line 21 at r3 (raw file):

Done.

libos/test/regression/rlimit_stack.c line 39 at r3 (raw file):

Done.

libos/test/regression/test_libos.py line 1103 at r3 (raw file):

Done.

Reviewed 2 of 2 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion, "fixup! " found in commit messages' one-liners

Reviewed all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion

Reviewed 2 of 2 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion

Reviewable status: complete! all files reviewed, all discussions resolved